pass in order terms as sorted to TermInSetQuery() #17714
Conversation
mkhludnev
requested review from
anasalkouz,
andrross,
ashking94,
Bukhtawar,
CEHENKLE,
cwperks,
dblock,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
|
❌ Gradle check result for aa495eb: Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
@mkhludnev thanks for adding this change! This looks right, but |
|
❌ Gradle check result for 4aba2a0: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 5808ce4: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17714 +/- ##
============================================
+ Coverage 72.29% 72.42% +0.13%
- Complexity 65900 66031 +131
============================================
Files 5350 5351 +1
Lines 306185 306229 +44
Branches 44373 44374 +1
============================================
+ Hits 221347 221789 +442
+ Misses 66670 66306 -364
+ Partials 18168 18134 -34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
❌ Gradle check result for ec7707c: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❕ Gradle check result for 703d3c4: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Signed-off-by: Mikhail Khludnev <[email protected]>
|
❌ Gradle check result for d725111: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Mikhail Khludnev <[email protected]>
server/src/test/java/org/opensearch/index/mapper/KeywordFieldTypeTests.java
Show resolved
Hide resolved
|
❌ Gradle check result for 5d5a85f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Mikhail Khludnev <[email protected]>
|
❌ Gradle check result for 2938244: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Mikhail Khludnev <[email protected]>
|
❌ Gradle check result for 38f93f7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
one more optimization in Lucene 10.3 apache/lucene#14425 |
There was a problem hiding this comment.
Thanks @mkhludnev for fixing this issue. Few comments to help improve the readability
server/src/main/java/org/opensearch/index/mapper/BytesRefsCollectionBuilder.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/BytesRefsCollectionBuilder.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/BytesRefsCollectionBuilder.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/BytesRefsCollectionBuilder.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/BytesRefsCollectionBuilder.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Mikhail Khludnev <[email protected]>
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-17714-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 032f4095cf797059e718d74fd2d337d95f8a09a9
# Push it to GitHub
git push --set-upstream origin backport/backport-17714-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.xThen, create a pull request where the |
|
@jainankitk thanks for merging this. Let's remove these redundant test #17902 (pardon, my intent wasn't clear), and then I'll proceed with backporting. |
Followup for #17714: Remove redundant tests (#17902) * Remove redundant tests Signed-off-by: Mikhail Khludnev <[email protected]> * Fix empty collection test Signed-off-by: Mikhail Khludnev <[email protected]> --------- Signed-off-by: Mikhail Khludnev <[email protected]>
…#17714) * pass in order terms as sorted to TermInSetQuery() Signed-off-by: Mikhail Khludnev <[email protected]> * slightly more elegant solution Signed-off-by: Mikhail Khludnev <[email protected]> * Attempting mocking TermInSetQ constructor. Signed-off-by: Mikhail Khludnev <[email protected]> * Handle ids as well. Signed-off-by: Mikhail Khludnev <[email protected]> * forbidden api Signed-off-by: Mikhail Khludnev <[email protected]> * make unnecessary method slow but correct. Signed-off-by: Mikhail Khludnev <[email protected]> * make unnecessary method slow but correct. Signed-off-by: Mikhail Khludnev <[email protected]> * Polish test coverage Signed-off-by: Mikhail Khludnev <[email protected]> * CHANGELOG.md Signed-off-by: Mikhail Khludnev <[email protected]> * assertThrows Signed-off-by: Mikhail Khludnev <[email protected]> * spotlessApply Signed-off-by: Mikhail Khludnev <[email protected]> * coverage tests and refactoring Signed-off-by: Mikhail Khludnev <[email protected]> * javadoc Signed-off-by: Mikhail Khludnev <[email protected]> * javadoc Signed-off-by: Mikhail Khludnev <[email protected]> * mark nocommit Signed-off-by: Mikhail Khludnev <[email protected]> * one more nocommit test Signed-off-by: Mikhail Khludnev <[email protected]> * forbidden api Signed-off-by: Mikhail Khludnev <[email protected]> * no commit for out of line tests Signed-off-by: Mikhail Khludnev <[email protected]> * Review Signed-off-by: Mikhail Khludnev <[email protected]> --------- Signed-off-by: Mikhail Khludnev <[email protected]> Signed-off-by: Mikhail Khludnev <[email protected]>
Followup for opensearch-project#17714: Remove redundant tests (opensearch-project#17902) * Remove redundant tests Signed-off-by: Mikhail Khludnev <[email protected]> * Fix empty collection test Signed-off-by: Mikhail Khludnev <[email protected]> --------- Signed-off-by: Mikhail Khludnev <[email protected]>
|
backport pr #17916 |
Pass in-order terms as Sorted into TermInSet
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
spin off from discussion https://forum.opensearch.org/t/avoid-re-sorting-when-initializing-terminsetquery/23865